Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Implement optional groups #3531

Merged
merged 1 commit into from
Apr 7, 2015
Merged

Conversation

jhass
Copy link
Contributor

@jhass jhass commented Mar 30, 2015

The lack of discussion in rubygems/bundler-features#68 and rubygems/bundler-features#59 is depressing, so here's some meat to get things going, hopefully.

* Add optional option to the group method, defaulting to false
* Exclude optional groups from the groups to be installed
* Add --with parameter to bundle install to install a group that would
* otherwise be excluded
* Error out if a group is listed in both, --with and --without
* Add bundle_with option to capistrano task
@@ -151,6 +151,8 @@ def check
Bundler.rubygems.security_policy_keys.join('|')
method_option "without", :type => :array, :banner =>
"Exclude gems that are part of the specified named group."
method_option "with", :type => :array, :banner =>
"Include gems that are part of the specified named group."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably say "specified optional group" here, to make it clear that with only applies to optional groups.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that but actually it does not only apply to optional groups. Applying it to a normal group has no effect but also doesn't throw an error, however applying it to a normal group that was previously excluded with without, removes it from the excluded groups.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. I read the rest of the diff after I made that comment. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did most reasoning in rubygems/bundler-features#68, which is why I spared it in the PRs description :)

@indirect
Copy link
Member

This looks like the most reasonable implementation of with that we've seen so far. :) I'm not sure how I feel about group :foo, :optional => true vs optional_group :foo.

How do you feel about this change, @bundler/core?

@TimMoore
Copy link
Contributor

TimMoore commented Apr 1, 2015

👍 thanks @jhass!

@jhass
Copy link
Contributor Author

jhass commented Apr 7, 2015

Alright, anything else you need from my side?

@indirect
Copy link
Member

indirect commented Apr 7, 2015

Don't think so! Was just waiting for consensus from the core team and haven't had time to circle back around to this pull yet. Thanks!

indirect added a commit that referenced this pull request Apr 7, 2015
@indirect indirect merged commit 13f44d1 into rubygems:master Apr 7, 2015
@jhass
Copy link
Contributor Author

jhass commented Apr 7, 2015

<3 Thanks!

I know, annoying question, but any rough ETA on when I can see this in a release?

@indirect
Copy link
Member

indirect commented Apr 7, 2015

I'd like to publish a prerelease within the next week, and then the final will be a week or two after all the known bugs are worked out.

On Tue, Apr 7, 2015 at 11:25 AM, Jonne Haß notifications@github.com
wrote:

<3 Thanks!

I know, annoying question, but any rough ETA on when I can see this in a release?

Reply to this email directly or view it on GitHub:
#3531 (comment)

@jhass
Copy link
Contributor Author

jhass commented Apr 7, 2015

Awesome news, thanks!

@rymohr
Copy link

rymohr commented Apr 7, 2015

The advantage of the --only approach is that it guarantees the listed groups are the only groups that will be installed. Ever. If you want to include new groups in your build you have to do so explicitly within your build process.

The :optional => true approach doesn't have the same purity. When you're adding new gems to a project you're usually not thinking about the obscure conditions of your build process. You're thinking about development, staging, and production. You're not thinking about the enterprise build you only run a few times a year that runs on a completely different platform. What happens if you add a group and forget to make it optional? You break the build.

The shift in control is subtle but I think the bundle install caller should have the ultimate say on what gets installed, instead of relying on flags within the Gemfile.

@salimane
Copy link
Contributor

@rymohr +1

@coilysiren coilysiren modified the milestone: Release Archive Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants